-
-
Notifications
You must be signed in to change notification settings - Fork 717
fix: fix csv file collections #3513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@Jasonzyt is attempting to deploy a commit to the NuxtLabs Team on Vercel. A member of the Team first needs to authorize it. |
commit: |
Thanks for the PR @Jasonzyt people: defineCollection({
type: "data",
source: "org/people.csv",
schema: z.object({
"name": z.string(),
"email": z.string().email(),
}),
}), It is important to create 1 to 1 and predictable mapping between files in content directory and documents in database. Splitting CSV files outside of collection source login, breaks this predictability. |
In my case, I only need single file CSV support. My PR can implement single CSV support, but tbh some logic needs to be optimized. |
I'll update your PR, |
@Jasonzyt Could you check and test the behavior? |
src/module.ts
Outdated
const { queries, hash } = generateCollectionInsert(collection, parsedContent) | ||
list.push([key, queries, hash]) | ||
} | ||
const { queries, hash } = generateCollectionInsert(collection, parsedContent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Iβm not sure why you reverted this.
Since CSV files contain multiple rows, and each row should be treated as an independent ParsedContent, they require a special handling process. Each CSV file should not be processed as a single ParsedContent.
In contrast, formats like Markdown/JSON/YML contain only one ParsedContent per file, so they must be handled differently.
And the facts prove this point: after installing the latest package, errors occurred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CSV and other formats must be handled seperately. Maybe we can find a better way to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to do this, Content now has defineCSVSource
which reads csv file and generate a document for each row
In my case const { data } = await useAsyncData("albumsMeta", () => {
return queryCollection("albumsMeta").order("updated", "DESC").all();
}); returns
This is exactly the bug mentioned by #3511 |
@Jasonzyt could you share your reproduction repository? or provide minimal one? |
Here's my repo: https://github.com/Jasonzyt/gallery |
@Jasonzyt There was a mistake in a string, should be good now. Try with |
I installed Edit: |
Please solve this problem urgently. I have projects to submit |
@Jasonzyt I checked with your projects and it works as expected with single file sources. try with Note: I had to remove styles and some other deps because I was facing issue with |
π Linked issue
#3511
β Type of change
π Description
Currently, CSV collections donβt behave as described in the documentation.
Instead of allowing each CSV file to contain multiple entries (as the documentation
states), the current implementation treats each CSV file as a single piece of content.
π Checklist